Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix imports of the "electron" module that are missing an Electron guard #4935

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Apr 12, 2024

Fix imports of the "electron" module that are missing an Electron guard

Pull Request Type

  • Build improvement

Related issue

Noticed them while working on #4931

Description

Currently some of our imports of the electron module are correctly guarded by IS_ELECTRON checks, others unfortunately are not which means the web webpack config (and the downstream Android one) need to tell webpack to ignore the electron imports. This pull request adds the guards in the places that there were missing.

I recommend turning on GitHub's Hide Whitespace setting while reviewing this pull request, especially for the src/renderer/store/modules/settings.js file.

Testing

Run yarn run pack:web, if it doesn't complain about not being able to bundle Electron, I didn't forget to add a guard.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.20.0

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 12, 2024
@absidue absidue changed the title Fix imports of the electron "module" that are missing an Electron guard Fix imports of the "electron" module that are missing an Electron guard Apr 12, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 12, 2024 16:40
Copy link
Member

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a yarn run pack:web to the linter ci too so we can make sure we dont merge things that break the web builds? (Even though we dont officially support the web builds)

- run: yarn run pack

@absidue
Copy link
Member Author

absidue commented Apr 13, 2024

Should we add a yarn run pack:web to the linter ci too so we can make sure we dont merge things that break the web builds? (Even though we dont officially support the web builds)

- run: yarn run pack

We could if you want.

@FreeTubeBot FreeTubeBot merged commit bb7d90b into FreeTubeApp:development Apr 14, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 14, 2024
@absidue absidue deleted the fix-is-electron branch April 14, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants